fix: light account 1271 signing #963
Conversation
adamegyed
requested review from
moldy530,
rthomare,
dancoombs,
mokok123 and
dphilipson
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| message, | ||
| }); | ||
|
|
||
| // We must use a public client, rather than an account client, to verify the message, because AA-SDK incorrectly attaches the account address as a "from" field to all actions taken by that client, including the `eth_call` used internally by viem's signature verifier logic. Per EIP-684, contract creation reverts on non-zero nonce, and the `eth_call`'s from field implicitly increases the nonce of the account contract, causing the contract creation to revert. |
There was a problem hiding this comment.
Is this a known thing that we're working on fixing?
There was a problem hiding this comment.
TBD from the clients team. We've raised this as an issue in the past, but I'm not sure what the plan for this is.
Looks like there were a few other places in the codebase just string-matching on signatures. Currently working on updating these, ideally we can fix all of the test cases. |
adamegyed
force-pushed
the
adam/fix-light-account-signing
branch
from
c5fb6fb
to
9855ccf
Compare
adamegyed
force-pushed
the
adam/fix-light-account-signing
branch
from
61063b4
to
11a377c
Compare
adamegyed
force-pushed
the
adam/fix-light-account-signing
branch
from
11a377c
to
7b90c69
Compare
adamegyed
changed the title
| ).toBe( | ||
| "0x007ecc361d63ab82d89faeecfb79d40127f376c1ef3d545aeec3578eadce9d0c405a4d1ae6177bdebdc8413065014f735ee98da9643cc0e25c07a7423b694f8ae71b" | ||
| ); | ||
| await publicClient.verifyMessage({ |
There was a problem hiding this comment.
I wrote this comment before realizing that the codebase is already doing things the way you did everywhere, so no need to change. But since I already wrote the comment, you get it anyways as a free FYI.
Nit: it's more idiomatic to write this assertion as
await expect(publicClient.verifyMessage({ ... })).resolves.toBe(true)That way gives a clearer error message in the case where the assertion fails.
Likewise throughout.
The EIP-712 Domain has been incorrect for both versions of LightAccount. This PR fixes it, and updates the tests to actually verify the signatures rather than just string-matching on the output.
Pull Request Checklist
yarn test)sitefolder, and guidelines for updating/adding docs can be found in the contribution guide)feat!: breaking change)yarn lint:check) and fix any issues? (yarn lint:write)PR-Codex overview
The focus of this PR is to update the
signWith1271Wrapperfunction in thebase.tsfile and add support for version-based signing methods.Detailed summary
signWith1271Wrapperfunction inbase.tsto accept aversionparameter.createLightAccountBasefunction.